Disposal#1039
Conversation
henrydingliu
requested review from
genedan,
jbogaardt and
kennethshsu
as code owners
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 0c1830d. Configure here.
| #get backend | ||
| #calculate disposal rate triangle | ||
| self.xp = X.get_array_module() | ||
| self.X_ = X.sort_index() |
There was a problem hiding this comment.
Sorted X_ mismatches disposal triangle
High Severity
fit stores a sorted triangle in self.X_ but builds disposal_rate_tri from the unsorted local X. Weights and super().fit then mix sorted self.X_.values with rates and ult.values tied to the original row order, so origins can be paired incorrectly whenever the index is not already sorted.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 0c1830d. Configure here.
| X = X.set_backend("numpy") | ||
| else: | ||
| X = X.copy().incr_to_cum() | ||
| X = X.copy() |
There was a problem hiding this comment.
Removed cumulative conversion in fit
Medium Severity
fit no longer calls incr_to_cum() on X after copying or setting the numpy backend. Disposal rates are cumulative paid (or similar) divided by ultimate; fitting on incremental values yields incorrect ratios and regression inputs for incremental triangles.
Reviewed by Cursor Bugbot for commit 0c1830d. Configure here.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## disposal_rate_method #1039 +/- ##
========================================================
- Coverage 89.25% 89.24% -0.02%
========================================================
Files 91 91
Lines 5242 5233 -9
Branches 666 666
========================================================
- Hits 4679 4670 -9
- Misses 394 396 +2
+ Partials 169 167 -2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
1 participant
Summary of Changes
Related GitHub Issue(s)
Additional Context for Reviewers
uv run pytest) and documentation changes (uv run jb build docs --builder=custom --custom-builder=doctest)Note
Medium Risk
Removing incr_to_cum on fit changes disposal-rate inputs for non-cumulative triangles, and ultimate/backend alignment in transform can alter forecasts if backends or shapes were previously inconsistent.
Overview
DisposalRate now validates
sample_weight(ultimate) againstXin bothfitandtransformvia sharedMethodBase.validate_weight, matching chainladder methods behavior when shapes may broadcast.Fit no longer forces
incr_to_cum()on the loss triangle before building disposal rates; it keeps a sorted copy asself.X_and uses that for weights, parentsuper().fit, and the terminal disposal concat. Transform setsultimate_from the ultimate triangle’s latest diagonal on the same array backend as the fittedX_, instead of takinglatest_diagonalwithout backend alignment.MethodBase.validate_weightis refactored to a@staticmethodwithTriangletype hints and a short docstring soDisposalRatecan call it without duplicating logic.Reviewed by Cursor Bugbot for commit 0c1830d. Bugbot is set up for automated code reviews on this repo. Configure here.